op-node: Execution Layer Sync #8968

trianglesphere · 2024-01-12T00:39:59Z

Description

This passes unsafe payloads directly the EngineController for immediate
insertion when Execution Layer sync is active. This tells the execution
client to sync to that target. Once the EL sync is complete, the last
unsafe payload is marked as safe. This is required when doing snap sync
because the EL does not have the pre-state required to do the engine
consolidation until the sync is complete.

Tests

I've manually tested this code & also have added an action test.

Metadata

op-node/rollup/driver/state.go

codecov · 2024-01-18T23:50:38Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (90fd9b6) 28.53% compared to head (81a7a87) 0.69%.

❗ Current head 81a7a87 differs from pull request most recent head 000ea84. Consider uploading reports for the commit 000ea84 to get more accurate results

Additional details and impacted files

@@             Coverage Diff             @@
##           develop   #8968       +/-   ##
===========================================
- Coverage    28.53%   0.69%   -27.85%     
===========================================
  Files          166      87       -79     
  Lines         7236    2155     -5081     
  Branches      1235     500      -735     
===========================================
- Hits          2065      15     -2050     
+ Misses        5050    2140     -2910     
+ Partials       121       0      -121

Flag	Coverage Δ
cannon-go-tests	`?`
chain-mon-tests	`?`
common-ts-tests	`?`
contracts-bedrock-tests	`0.69% <ø> (ø)`
contracts-ts-tests	`?`
core-utils-tests	`?`
sdk-next-tests	`?`
sdk-tests	`?`

Flags with carried forward coverage won't be shown. Click here to find out more.

see 79 files with indirect coverage changes

op-e2e/actions/l2_verifier.go

op-program/client/l2/engineapi/l2_engine_api.go

semgrep-app · 2024-01-18T23:54:08Z

Semgrep found 10 sol-style-return-arg-fmt findings:

packages/contracts-bedrock/scripts/Deployer.sol: L667, L665, L658, L627, L306, L295, L234, L117, L104
packages/contracts-bedrock/scripts/DeployL2.s.sol: L20

Named return arguments to functions must be appended with an underscore (_)

_{Ignore this finding from sol-style-return-arg-fmt.}

coderabbitai · 2024-01-19T01:11:35Z

Walkthrough

The changes involve integrating P2P (peer-to-peer) functionality and enhancing synchronization features across several components of a blockchain-related codebase. New methods and configurations have been added to handle P2P connections, sync status, and unsafe payload insertion. Modifications to the test suites suggest that these features are also being verified for correctness and stability. The changes indicate a focus on networking capabilities and data consistency within the system.

Changes

Files	Change Summary
`op-e2e/actions/l2_engine.go` `op-e2e/actions/l2_verifier.go` `op-node/node/node.go` `op-node/rollup/derive/engine_controller.go` `op-node/rollup/driver/driver.go` `op-node/rollup/driver/state.go` `op-program/client/l2/engine.go` `op-program/client/l2/engineapi/l2_engine_api.go`	Enhanced P2P and synchronization features, including new methods for node interaction and status checking. Adjusted function signatures to accommodate new arguments related to P2P and sync configurations.
`op-e2e/actions/l2_sequencer_test.go` `op-e2e/actions/l2_verifier_test.go` `op-e2e/actions/sync_test.go` `op-e2e/system_test.go` `op-program/client/l2/engineapi/test/l2_engine_api_tests.go`	Updated tests to include P2P settings and synchronization checks. Added imports for Ethereum packages and time handling, and introduced new parameters and assertions.
`op-node/rollup/derive/engine_controller.go` `op-node/rollup/driver/state.go`	Introduced a new `syncStatusEnum` type with associated constants and logic to handle different synchronization states.

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share

Tips

Chat

There are 3 ways to chat with CodeRabbit:

Review comments: Directly reply to a review comment made by CodeRabbit. Example:
- I pushed a fix in commit <commit_id>.
- Generate unit-tests for this file.
Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
- @coderabbitai generate unit tests for this file.
- @coderabbitai modularize this function.
PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
- @coderabbitai generate interesting stats about this repository from git and render them as a table.
- @coderabbitai show all the console.log statements in this repository.
- @coderabbitai read src/utils.ts and generate unit tests.
- @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (invoked as PR comments)

@coderabbitai pause to pause the reviews on a PR.
@coderabbitai resume to resume the paused reviews.
@coderabbitai review to trigger a review. This is useful when automatic reviews are disabled for the repository.
@coderabbitai resolve resolve all the CodeRabbit review comments.
@coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (`.coderabbit.yaml`)

You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
The JSON schema for the configuration file is available here.
If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/coderabbit-overrides.v2.json

CodeRabbit Discord Community

Join our Discord Community to get help, request features, and share feedback.

ajsutton

LGTM.

op-e2e/actions/sync_test.go

op-node/rollup/derive/engine_controller.go

op-program/client/l2/engineapi/l2_engine_api.go

protolambda

Generally looks good, but I am wondering how we exit a syncing engine status when there is no derivation or p2p activity.

op-e2e/actions/l2_sequencer_test.go

op-node/rollup/driver/state.go

semgrep-app · 2024-01-19T16:40:58Z

Semgrep found 2 sol-style-return-arg-fmt findings:

packages/contracts-bedrock/scripts/universal/SafeBuilder.sol: L189, L42

Named return arguments to functions must be appended with an underscore (_)

_{Ignore this finding from sol-style-return-arg-fmt.}

Semgrep found 1 sol-style-malformed-require finding:

packages/contracts-bedrock/scripts/universal/SafeBuilder.sol: L173

Malformed require statement style.

_{Ignore this finding from sol-style-malformed-require.}

This passes unsafe payloads directly the EngineController for immediate insertion when Execution Layer sync is active. This tells the execution client to sync to that target. Once the EL sync is complete, the last unsafe payload is marked as safe. This is required when doing snap sync because the EL does not have the pre-state required to do the engine consolidation until the sync is complete.

* op-node: Execution Layer Sync This passes unsafe payloads directly the EngineController for immediate insertion when Execution Layer sync is active. This tells the execution client to sync to that target. Once the EL sync is complete, the last unsafe payload is marked as safe. This is required when doing snap sync because the EL does not have the pre-state required to do the engine consolidation until the sync is complete. * op-e2e: Snap Sync action test * op-e2e: Actually run TestSystemE2E * op-service/node: Add time.Since to Clock & use it

trianglesphere force-pushed the jg/el_sync branch from 9a6deba to c72087a Compare January 12, 2024 01:52

trianglesphere force-pushed the jg/pass_in_engine_controller branch 2 times, most recently from 812932c to 96024d1 Compare January 13, 2024 00:10

trianglesphere force-pushed the jg/el_sync branch from c72087a to bda0aed Compare January 13, 2024 00:13

semgrep-app bot reviewed Jan 13, 2024

View reviewed changes

op-node/rollup/driver/state.go Outdated Show resolved Hide resolved

trianglesphere force-pushed the jg/pass_in_engine_controller branch from 96024d1 to fe72b65 Compare January 13, 2024 00:22

trianglesphere force-pushed the jg/el_sync branch from bda0aed to 77c8d14 Compare January 13, 2024 00:22

trianglesphere force-pushed the jg/pass_in_engine_controller branch from fe72b65 to 897fa18 Compare January 13, 2024 00:24

trianglesphere force-pushed the jg/el_sync branch from 77c8d14 to 1c5bb32 Compare January 13, 2024 00:24

trianglesphere force-pushed the jg/pass_in_engine_controller branch from 897fa18 to 4077200 Compare January 13, 2024 00:25

trianglesphere force-pushed the jg/el_sync branch from 1c5bb32 to ef9d9fe Compare January 13, 2024 00:27

pcw109550 mentioned this pull request Jan 15, 2024

op-node: Restore previous unsafe chain when invalid span batch #8925

Merged

trianglesphere force-pushed the jg/pass_in_engine_controller branch 2 times, most recently from 26f824a to a7c3286 Compare January 17, 2024 03:07

Base automatically changed from jg/pass_in_engine_controller to develop January 17, 2024 05:24

trianglesphere force-pushed the jg/el_sync branch from ef9d9fe to bb04c92 Compare January 18, 2024 23:50

semgrep-app bot reviewed Jan 18, 2024

View reviewed changes

op-e2e/actions/l2_verifier.go Outdated Show resolved Hide resolved

semgrep-app bot reviewed Jan 18, 2024

View reviewed changes

op-program/client/l2/engineapi/l2_engine_api.go Outdated Show resolved Hide resolved

trianglesphere force-pushed the jg/el_sync branch 5 times, most recently from 0aa9f05 to 155d58c Compare January 19, 2024 01:09

trianglesphere marked this pull request as ready for review January 19, 2024 01:11

trianglesphere requested review from a team, protolambda and ajsutton as code owners January 19, 2024 01:11

trianglesphere force-pushed the jg/el_sync branch 2 times, most recently from b376643 to 4286981 Compare January 19, 2024 01:30

ajsutton approved these changes Jan 19, 2024

View reviewed changes

op-e2e/actions/sync_test.go Outdated Show resolved Hide resolved

op-node/rollup/derive/engine_controller.go Outdated Show resolved Hide resolved

op-program/client/l2/engineapi/l2_engine_api.go Show resolved Hide resolved

protolambda reviewed Jan 19, 2024

View reviewed changes

op-e2e/actions/l2_sequencer_test.go Outdated Show resolved Hide resolved

op-node/rollup/driver/state.go Show resolved Hide resolved

trianglesphere force-pushed the jg/el_sync branch from 4286981 to 32cb7a8 Compare January 19, 2024 16:36

trianglesphere added this pull request to the merge queue Jan 19, 2024

github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jan 19, 2024

trianglesphere added this pull request to the merge queue Jan 19, 2024

github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jan 19, 2024

trianglesphere force-pushed the jg/el_sync branch from 32cb7a8 to 81a7a87 Compare January 19, 2024 17:23

trianglesphere added 4 commits January 19, 2024 09:26

op-e2e: Snap Sync action test

d966b46

op-e2e: Actually run TestSystemE2E

9aef759

op-service/node: Add time.Since to Clock & use it

000ea84

trianglesphere force-pushed the jg/el_sync branch from 81a7a87 to 000ea84 Compare January 19, 2024 17:26

trianglesphere enabled auto-merge January 19, 2024 17:33

trianglesphere added this pull request to the merge queue Jan 19, 2024

github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jan 19, 2024

trianglesphere added this pull request to the merge queue Jan 19, 2024

Merged via the queue into develop with commit 770d37f Jan 19, 2024
66 checks passed

trianglesphere deleted the jg/el_sync branch January 19, 2024 18:32

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

op-node: Execution Layer Sync #8968

op-node: Execution Layer Sync #8968

trianglesphere commented Jan 12, 2024 •

edited

Loading

codecov bot commented Jan 18, 2024 •

edited

Loading

semgrep-app bot commented Jan 18, 2024

coderabbitai bot commented Jan 19, 2024 •

edited

Loading

Walkthrough

Changes

Chat

CodeRabbit Commands (invoked as PR comments)

CodeRabbit Configration File (`.coderabbit.yaml`)

CodeRabbit Discord Community

ajsutton left a comment

protolambda left a comment

semgrep-app bot commented Jan 19, 2024

op-node: Execution Layer Sync #8968

op-node: Execution Layer Sync #8968

Conversation

trianglesphere commented Jan 12, 2024 • edited Loading

codecov bot commented Jan 18, 2024 • edited Loading

Codecov Report

semgrep-app bot commented Jan 18, 2024

coderabbitai bot commented Jan 19, 2024 • edited Loading

Walkthrough

Changes

Chat

CodeRabbit Commands (invoked as PR comments)

CodeRabbit Configration File (.coderabbit.yaml)

CodeRabbit Discord Community

ajsutton left a comment

Choose a reason for hiding this comment

protolambda left a comment

Choose a reason for hiding this comment

semgrep-app bot commented Jan 19, 2024

trianglesphere commented Jan 12, 2024 •

edited

Loading

codecov bot commented Jan 18, 2024 •

edited

Loading

coderabbitai bot commented Jan 19, 2024 •

edited

Loading

CodeRabbit Configration File (`.coderabbit.yaml`)